Repository navigation
Conversation
Recognize lhm wrappers while preserving custom-hook safeguards and inherited configuration. Run Buzz first for commits, lhm first for pushes, replay push input, and forward other hook events. Document recovery and cover installation, ordering, failure, signal, and stdin contracts with integration tests. Signed-off-by: Matt Toohey <contact@matttoohey.com>
Replace the worktree-local dispatcher, installer, custom check-staged and check-push groups and .githooks/ with ordinary pre-commit and pre-push jobs in lefthook.yml. lhm merges them with the machine policy at hook time, so there is nothing to install; without lhm, `bin/lefthook install` runs once per clone. Pin Lefthook 2.1.16, where a partial-staging restore conflict no longer discards unrelated unstaged edits, and require it through min_version. Lefthook now owns partial staging instead of the hook refusing it. Pre-commit is piped and opens with a read-only guard that refuses staged names Git would expand as globs: Lefthook restages with `git add --force -- <name>`, so a staged `a[1].ts` would otherwise sweep `a1.ts` into the commit. check-icons.mjs accepts an explicit file list for the staged job. Dropped with the dispatcher: refusing partially staged files, the unstaged formatter-configuration check and the symlink type-change case; CI formats with committed configuration. Tests cover installation, linked worktrees, both partial-staging paths, failure ordering, push stdin and real lhm composition. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
matt2e
changed the title
wesbillman
left a comment
wesbillman
left a comment
There was a problem hiding this comment.
Changes needed: the new partial-staging path shares recovery state across linked worktrees, risking loss of unstaged edits during concurrent commits (P1 inline).
Star Lord automated source review via Wes’s account. Head 2e6d83c7517cd07c24659c57d16b291b19aeffe2; base 27d581e5ce0dc5bd6eb6cd09f6dd0c22545f4306. No code, hooks, tests, builds, or apps executed. Hosted automatic CI passed, but its real-lhm case was skipped; the PR’s human commit/push acceptance remains pending.
| # skipping when they cannot find Lefthook. lhm users install nothing: lhm merges | ||
| # this file with the machine policy at hook time. | ||
| assert_lefthook_installed: true | ||
| pre-commit: |
There was a problem hiding this comment.
P1 — Isolate partial-staging recovery across linked worktrees
Switching to the standard hook enables Lefthook’s automatic hide/restore guard. In pinned 2.1.16, the recovery directory comes from git rev-parse --git-path info, which is shared by linked worktrees. Both patch files have fixed names, and cleanup drops every matching automatic backup, not just this invocation’s stash.
Two developers/agents committing partially staged changes in separate worktrees can therefore overwrite/remove each other’s recovery patches and backups, leaving hidden edits unrestored. The new linked-worktree test only makes a single commit, so it does not cover this supported workflow.
Serialize the complete pre-commit operation across the clone before Lefthook hides edits, or require a runner fix with worktree-local recovery files and invocation-specific backup cleanup. Add deterministic coverage for overlapping partial commits in two linked worktrees, verifying both unstaged edits and existing stashes survive.
Signed-off-by: Star Lord <b89298dbe87c6b3fd8a425b535d9d161f23c88d555a47925ae0b04dfe02b201e@buzz.block.builderlab.xyz>
Signed-off-by: Star Lord <b89298dbe87c6b3fd8a425b535d9d161f23c88d555a47925ae0b04dfe02b201e@buzz.block.builderlab.xyz>
wesbillman
left a comment
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
No remaining code blockers found at 88566880. The previous concurrent-worktree P1 is addressed by the pinned recovery patch. This is a comment review, not approval.
- Reviewed head
88566880cad9f57570cc140b2e4134d9fba57ae6against base27d581e5ce0dc5bd6eb6cd09f6dd0c22545f4306, with an independent recovery/ownership source review. At that clean head, all 39/39 hook integration cases passed, no skips, on macOS arm64 with real lhm 0.14.1 and stock-runner rejection enabled. This includes overlapping commits, restoration failure, GC retention, unchanged legacy stashes, and machine-policy composition. No application code changed during review. - CI is not green: the JavaScript job fails on missing
mentionCandidatesinMessageComposer.tsx. CI checked mergeecc948f016c904ae04855e65a82c1b8c7cfe92db; that file is byte-identical to its main parent1679c78cd36ae6bb8bd057b47497ba344ebcb9c4, so this is not introduced by the hook changes. Other checks were still running at the snapshot. - Remaining gates: green required CI and the documented human commit/push acceptance. The lhm Hermit-PATH requirement and future stock-version gate limitation remain explicit operating constraints. I did not independently rerun the full upstream Go suites or validate other platforms. One optional recovery-diagnostic improvement is inline.
| r.logger.Warn( | ||
| "Saved unstaged changes not found. " + | ||
| - "Restore them from the 'lefthook auto backup' stash: git stash list", | ||
| + "List recovery refs with git for-each-ref refs/lefthook/backup/ " + |
There was a problem hiding this comment.
P3, optional: identify this run’s recovery ref in failure output.
The missing-patch warning lists the whole backup namespace, while an apply failure still returns only the patch error. Since these backups no longer appear in git stash list, include refs/lefthook/backup/<backup> in both paths and point to the documented safe recovery procedure (preserve current edits, then apply with --index at the original base). That makes retained edits discoverable without guessing among old backups. This is not a blocker: the ref is retained and the corruption/GC/recovery integration cases pass.
Signed-off-by: Star Lord <b89298dbe87c6b3fd8a425b535d9d161f23c88d555a47925ae0b04dfe02b201e@buzz.block.builderlab.xyz>
wesbillman
left a comment
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Follow-up code review: no new blockers. Merge readiness remains pending CI and human hook acceptance; this is not approval.
Reviewed head 12d96a08ee07d8075e91a08aafbe6e5f5ab0739e against base b03be61259dcdef36a6ee655cd3e596bdcd3faec, focusing on the delta since my previous review.
- The only new first-parent commit is the main merge. Its tree exactly matches Git’s automatic merge; all hook implementation, package/patch, configuration and hook-test files are unchanged from
88566880. The three overlapping documentation files preserve both sets of changes. Prior recovery findings and the optional diagnostic suggestion are unchanged. - Main’s #684 repair is included unchanged: the undefined composer call is replaced with the imported
pastedMentionRecipienthelper. In the current CI snapshot, both JavaScript lint/type and frontend-build steps pass; JavaScript shard 1, browser measurements, native fixture, DCO and security checks have passed. Other test lanes remain in progress. This is not an all-green result. - No local suites rerun for this merge-only follow-up. My prior 39/39 hook result applies to
88566880; the PR separately reports 198/198 Node integration cases at the new head. Before merge: finish required current-head CI and the documented human commit/push acceptance, plus required reviewer approval.
3 participants
Summary
Use standard Lefthook pre-commit/pre-push hooks so lhm composes repository checks with machine policy. Without lhm,
just hooksinstalls once per clone, including linked worktrees. Remove the custom dispatcher/installer while preserving staged formatting, failure checks and push gates.The concurrent partial-staging P1 is repaired in
88566880. The owner marked this PR ready for review; merge readiness still needs current hosted checks, required approval and human hook acceptance. Runner implementation and local validation are complete.Recovery repair
2.1.18-buzz.3, built by Hermit from checksum-pinned public upstream source plus the included MIT-licensed patch. No global runner replacement or separate release service.refs/lefthook/backup/<hash>refs; delete only the current owner's ref after restoration succeeds. Failed restoration remains recoverable after GC; existing user/legacy stashes stay untouched.bin/lefthook; lhm uses the activated Hermit PATH. Current stock runners through 2.1.17 are rejected before hiding unstaged changes.Migration and limits
source bin/activate-hermit), verifycommand -v lefthookpoints to this checkout'sbin/lefthook, and install nothing. GUI clients must actually inherit that environment. lhm 0.14.1 can silently skip hooks if no runner is on PATH; repository configuration cannot fix its fallback.just hooksonce per clone, not once per linked worktree. Old standalone shims cannot self-update through the new version gate.core.hooksPathoverrides from the previous installer. Preserve machine/global hooks; the contribution guide documents an explicit non-lhm clone-local override and warns against reset/unset-global fixes.git push --mirror.min_versionis not a capability check: future stock 2.1.18+ would pass. Revalidate upstream before replacing this pin. Provenance, recovery, independent tests and removal criteria:bin/packages/lefthook-recovery.md.Validation
Latest update: merged main
b03be612(including the #684 pasted-mention repair) without rewriting existing commits. Current PR head:12d96a08. The hook implementation is unchanged by this merge.12d96a08, full Node integration: 198/198 passed, no skips, macOS arm64, pinned Node, file concurrency 2, real lhm 0.14.1 and stock-runner rejection enabled. Includes the previously failing page-build tests, overlapping worktree commits, failed-restore recovery after GC, unchanged legacy stashes and machine-policy composition.mentionCandidatesfailure is addressed by main fix(messages): restore main typecheck for pasted mentions #684, now included. At the new-head hosted snapshot, DCO passed and the fresh CI jobs were queued; no green-CI claim yet.Human acceptance still required
In disposable lhm and standalone checkouts, exercise commit/push: fully staged unformatted source must land formatted; a partial commit must contain only staged hunks and leave
git stash listunchanged; a Biome warning must reject the commit without changing the index. Automated fixtures do not substitute for human confirmation. These checks and the required current-head checks/approval remain merge-readiness gaps; the owner's ready-for-review setting has been preserved.